-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[Monitor OpenTelemetry] Add Ability to Config Views
#36684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[Monitor OpenTelemetry] Add Ability to Config Views
#36684
Conversation
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for configuring custom metric views in the Azure Monitor OpenTelemetry SDK through a new views property in AzureMonitorOpenTelemetryOptions. Users can now provide custom view configurations that will be merged with default views and passed to the OpenTelemetry NodeSDK's meter provider.
Key changes:
- Added
viewsproperty toAzureMonitorOpenTelemetryOptionsfor custom metric view configuration - Custom views are merged with default views from
MetricHandlerbefore SDK initialization - Improved test isolation in trace handler tests by tracking and properly managing instrumentation lifecycle
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sdk/monitor/monitor-opentelemetry/src/types.ts | Added ViewOptions[] import and views property to AzureMonitorOpenTelemetryOptions interface |
| sdk/monitor/monitor-opentelemetry/src/index.ts | Implemented merging of custom views with default views and passed them to NodeSDK configuration |
| sdk/monitor/monitor-opentelemetry/test/internal/unit/main.test.ts | Added unit test to verify custom metric views are registered with the meter provider |
| sdk/monitor/monitor-opentelemetry/test/snippets.spec.ts | Updated snippet test to include empty views array in sample configuration |
| sdk/monitor/monitor-opentelemetry/test/internal/unit/traces/traceHandler.test.ts | Improved test reliability by properly managing instrumentation lifecycle (enable/disable/setTracerProvider) |
| sdk/monitor/monitor-opentelemetry/review/monitor-opentelemetry-node.api.md | Added ViewOptions import and views property to API surface |
| sdk/monitor/monitor-opentelemetry/README.md | Documented the new views option in usage examples and configuration table |
| sdk/monitor/monitor-opentelemetry/CHANGELOG.md | Added changelog entry for version 1.15.0 describing the new feature |
| azureMonitorExporterOptions: { | ||
| connectionString: "InstrumentationKey=00000000-0000-0000-0000-000000000000", | ||
| }, | ||
| views: [customView], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible to have multiple custom views right? In that case, does config have the ability to process them as a list of views?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we just pass this to the NodeSDKConfiguration in OpenTelemetry which takes a ViewOptions array.
| <td></td> | ||
| </tr> | ||
| <tr> | ||
| <td><code>views</code></td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the documentation for the configure opentelemetry needs to be updated for this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it will need to be, but waiting on PR review & API review.
rads-1996
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Packages impacted by this PR
@azure/monitor-opentelemetry
Issues associated with this PR
#33012 (comment)
Describe the problem that is addressed by this PR
This pull request introduces support for configuring custom metric views in the Azure Monitor OpenTelemetry SDK. Users can now specify additional metric views via the
viewsproperty inAzureMonitorOpenTelemetryOptions, which will be registered with the NodeSDK's meter provider. The change is documented in the README and changelog, and is covered by new unit tests to ensure correct behavior.Custom metric views support:
viewsproperty (array ofViewOptions) to theAzureMonitorOpenTelemetryOptionsinterface insrc/types.tsand updated all relevant type imports. [1] [2] [3] [4]src/index.tsto merge user-provided views with default views and pass them to the NodeSDK configuration. [1] [2]viewsoption in the README usage example and options table. [1] [2]Testing and validation:
viewsoption in sample usage.Test suite improvements:
Checklists